Codecov Report

Merging #700 (dc3cd88) into main (161e7e3) will decrease coverage by 0.87%.
The diff coverage is 17.52%.

@@            Coverage Diff             @@
##             main     #700      +/-   ##
==========================================
- Coverage   57.23%   56.37%   -0.87%     
==========================================
  Files         290      293       +3     
  Lines        6681     6954     +273     
  Branches      907      954      +47     
==========================================
+ Hits         3824     3920      +96     
- Misses       2691     2844     +153     
- Partials      166      190      +24     

Flags with carried forward coverage won't be shown. Click here to find out more.

... and 15 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@aaronchongth
schedule package provides the clear function that receives a job tag.
I have added a tag to the job in the to_job() function which is the id of the schedule.
clear function removes jobs by iterating through each job using its tag which is a Hashable.

Both schedule.clear() and schedule.clean_job() functions iterate through a list of jobs.

This has O(n) complexity, which implies that the execution time when deleting schedules is proportional to the size of the input. It doesn't seem very inefficient to me unless we are considering a large amount of data.

The api-server error is because pydantic warning Access to a protected member _id of a client class (protected-access)
Why we called _id instead of id here ?

@Angatupyry, thank you so much for the help! I can verify that the deletion bug has been fixed.

This has O(n) complexity, which implies that the execution time when deleting schedules is proportional to the size of the input. It doesn't seem very inefficient to me unless we are considering a large amount of data.

Yup, I'm willing to live with that. We can further optimize this when we implement end-date for schedules, and have to handle schedules that have already past.

The api-server error is because pydantic warning Access to a protected member _id of a client class (protected-access)
Why we called _id instead of id here ?

Gotcha! I was actually referring to a stalling error, but it looks like it may have already been resolved, perhaps it was a github runner issue. The member is called _id to be considered a protected member of the Model, and hence will the interface will not include the member, https://github.com/open-rmf/rmf-web/blob/kp/scheduling-frontend/packages/api-client/lib/openapi/api.ts#L299-L330

I've added a getter to access the protected member instead, this resolves the linting error

I'm liking how this is looking! Thanks for all the help so far @koonpeng and @Angatupyry! LGTM

@Angatupyry can you do another round of review for this? Probably not a good idea for me to approve and just merge what I wrote 🤣

haha! Thanks, Aaron!
I will!

@aaronchongth

The only thing I could say is that maybe it doesn't make much sense that the start date could be less than the current date.
Perhaps it could be restricted as a minimum acceptable to today.
But it is a detail that does not affect functionality.

Yeah I understand what you mean. Well in the spirit of mimicking a modern calendar system, we should also allow handling of schedules in the past then. Thanks for the work and review from all! Merging it! 💥